-
Notifications
You must be signed in to change notification settings - Fork 62
feat: add gunicorn_access_logformat field to Api and Content specs #1578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add gunicorn_access_logformat field to Api and Content specs #1578
Conversation
|
Hi @yaker-gh. Thanks for your PR. I'm waiting for a pulp member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
67635e4 to
78cde5b
Compare
|
@git-hyagi I am not entirely sure why the CI is failing. 🤔 Do you have any idea? |
|
Hi @balasankarc! hmm... in this PR we are trying to define the same default log format for API and content pods, but they use different gunicorn worker classes (which accept different log formats). What if we define the aiohttp's default log format for the content pods? |
@git-hyagi Oops. I had figured that out (and commented above) after I pinged you. 😅 Sorry about the noise. |
|
@git-hyagi When debugging CI failures like this, what's your usual approach? I spent some time going through the logs but wasn't sure which pods/steps to prioritize. Is there documentation for the CI pipeline structure, or is this mostly standard k8s deployment patterns I should study up on? |
|
Hi @yaker-gh
I usually start by checking the failed job, the operator logs, the pods statuses, and Pulp logs. For example, in this case, I saw that some tests worked: it created the Pulp to get a better understanding of why if failed to download the image, I checked the pods statuses (and we probably found a bug in the probes' configuration because all pods were considered READY): after that I checked the API and content logs:
No 😢 we didn't document our CI workflow, most of the steps are shell commands (a mix of make, kubectl, and .sh execution) and they are split as GH jobs, like:
|
|
Thanks, @git-hyagi. That's insightful and immensely helpful. |
* Pass gunicorn_access_logformat as env var and use in container args * Add gunicorn_access_logformat field to Api and Content specs * Add changelog * Update controller_test.go with new env vars and args * Run `make manifests` and `make bundle` to update CRDs closes pulp#1564
78cde5b to
1c0951a
Compare
I deployed the change in a GKE instance, and everything seems to be working fine. I think this one is good to go. Thanks @yaker-gh |
|
/ok-to-test |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: balasankarc, yaker-gh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@git-hyagi We still need your approval. 😁 |
Spoke too soon. I could merge. Sorry, GitHub UI is very confusing, and openshift bot added to that. 😅 |



Summary
Adds a new CRD field
gunicorn_access_logformatto customize gunicorn's access log format for api and content pods.Closes #1564
Changes
pulp_types.go
GunicornAccessLogformatfield toApistructGunicornAccessLogformatfield toContentstructdeployment.go
setEnvVars(): SetPULP_GUNICORN_ACCESS_LOGFORMATenv var with default value if field is emptypulpcoreApiContainerArgs(): Remove hardcoded--access-logformatfrom gunicorn ENTRYPOINT array, add--access-logformat "${PULP_GUNICORN_ACCESS_LOGFORMAT}"to exec commandpulpcoreContentContainerArgs(): Add--access-logformat "${PULP_GUNICORN_ACCESS_LOGFORMAT}"to exec commandcontroller_test.go
PULP_GUNICORN_ACCESS_LOGFORMATtoenvVarsApiandenvVarsContentapiContainersand content container Args to match new shell script behaviorDefault format:
This preserves existing behavior when the field is not specified - the default includes the correlation-id header from django_guid.
Usage